Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve assertions by showing the last result on a timeout #13119

Merged
merged 3 commits into from
Feb 4, 2025

Conversation

zeitlinger
Copy link
Member

@zeitlinger zeitlinger commented Jan 28, 2025

When log and metrics assertions fail, they don't show which assertion actually failed - you just see "timed out" - even the stack trace is lost.

This PR restores the stack trace by running the assertion one last time in the thread of the caller.

In addition, we now also tell the user which metric was not found.

@zeitlinger zeitlinger self-assigned this Jan 28, 2025
@zeitlinger zeitlinger requested a review from a team as a code owner January 28, 2025 18:01
@zeitlinger
Copy link
Member Author

@steverao can you check again?

@@ -125,12 +126,9 @@ public List<List<SpanData>> waitForTraces(int numberOfTraces) {
* This waits up to 20 seconds, then times out.
*/
public List<LogRecordData> waitForLogRecords(int numberOfLogRecords) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firstly I think you should update the PR title or description to make it easier to understand what you are doing. As far as I can tell the improvement is that you are making metric and log assertion similar to trace assertion.
WDYT about moving the log assertion logic to InstrumentationTestRunner as it is for traces/metrics. Then you would not need the AwaitUtil class and could have the helper method in InstrumentationTestRunner.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point @laurit - fixed

@laurit laurit merged commit ff38437 into open-telemetry:main Feb 4, 2025
59 checks passed
@zeitlinger zeitlinger deleted the await-util branch February 5, 2025 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants